Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(dev-env): let dev-tools handle their volumes #2139

Merged
merged 2 commits into from
Dec 12, 2024
Merged

Conversation

sjinks
Copy link
Member

@sjinks sjinks commented Dec 5, 2024

Description

This PR updates the dev-env code to avoid messing with the dev-tools volume.

  1. We no longer remove dev-tools volumes;
  2. We ensure that the php service starts after devtools completes to avoid race conditions;
  3. We sync /dev-tools-orig with /dev-tools. This step is necessary because Lando sets its own entrypoint script if we don't provide ours.

Related PR: Automattic/vip-container-images#985 (must be merged before this one)
Volume removal logic was introduced in #1317.

Pull request checklist

New release checklist

Steps to Test

  1. Check out refactor(dev-tools): always update /devtools vip-container-images#985, cd dev-tools && docker build -t dev-tools:local
  2. Check out this PR, npm run build && npm link
  3. vip dev-env create < /dev/null
  4. Create ~/.local/share/vip/dev-environment/vip-local/.lando.local.yml:
services:
  devtools:
    services:
      image: dev-tools:local
      pull_policy: never
  1. Run vip dev-env start and ensure everything works
  2. Run vip dev-env shell -- sh -c 'md5sum /dev-tools/*' and save the output.
  3. Update one of the dev-tools scripts and rebuild the image (docker build -t dev-tools:local)
  4. Restart the environment (vip dev-env start).
  5. Run vip dev-env shell -- sh -c 'md5sum /dev-tools/*' and compare the output with the one from step 6. The hash for the updated script must differ.

@sjinks sjinks requested a review from rinatkhaziev December 5, 2024 05:30
@sjinks sjinks self-assigned this Dec 5, 2024
Copy link
Contributor

github-actions bot commented Dec 5, 2024

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Files

@@ -31,6 +30,7 @@ services:
devtools: {}
scripts:
initOnly: true
entrypoint: sh -c 'test -d /dev-tools-orig && /usr/bin/rsync -a --delete /dev-tools-orig/ /dev-tools/ || true'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Backward compatibility :-(

@sjinks sjinks changed the title refactor(dev-env): let dev-tools handle their volume refactor(dev-env): let dev-tools handle their volumes Dec 6, 2024
@rinatkhaziev
Copy link
Contributor

I've tested both for a) existing environment and b) fresh env under Mac OS X

  1. The good news is that files do get updated on image rebuild.
  2. The bad news that on environment start nginx quits with
2024-12-06 11:15:25 lando 17:15:25.INFO  ==> Lando handing off to: nginx -g daemon off;
2024-12-06 11:15:25 lando 17:15:25.DEBUG ==> Running command with exec...
2024-12-06 11:15:25 2024/12/06 17:15:25 [emerg] 1#1: host not found in upstream "php" in /etc/nginx/conf.d/default.conf:60
2024-12-06 11:15:25 nginx: [emerg] host not found in upstream "php" in /etc/nginx/conf.d/default.conf:60

Manual start of nginx container after that works.

@sjinks
Copy link
Member Author

sjinks commented Dec 7, 2024

Updated service dependencies.

@sjinks sjinks force-pushed the dev-env-volume-fun branch from 6d13d47 to f77bded Compare December 12, 2024 08:46
Copy link

sonarcloud bot commented Dec 12, 2024

@rinatkhaziev
Copy link
Contributor

Tested, works!

@sjinks sjinks merged commit c12f9d4 into trunk Dec 12, 2024
17 checks passed
@sjinks sjinks deleted the dev-env-volume-fun branch December 12, 2024 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants